Skip to content

GH-46499: [CI][Crossbow][C++] Use apache/arrow for Meson#46501

Merged
kou merged 1 commit into
apache:mainfrom
kou:ci-crossbow-cpp-meson
May 30, 2025
Merged

GH-46499: [CI][Crossbow][C++] Use apache/arrow for Meson#46501
kou merged 1 commit into
apache:mainfrom
kou:ci-crossbow-cpp-meson

Conversation

@kou

@kou kou commented May 19, 2025

Copy link
Copy Markdown
Member

Rationale for this change

We're using Crossbow to offload CI jobs to separated repository but it has some inconveniences. See #46014 for details.

What changes are included in this PR?

Use CI: Extra label to run Meson CI job as an extra CI job.

Are these changes tested?

Yes.

Are there any user-facing changes?

No.

@kou kou added the CI: Extra Run extra CI label May 19, 2025
@github-actions

Copy link
Copy Markdown

⚠️ GitHub issue #46499 has been automatically assigned in GitHub to PR creator.

@github-actions github-actions Bot added the awaiting committer review Awaiting committer review label May 19, 2025
@kou kou force-pushed the ci-crossbow-cpp-meson branch from 36044ff to db197ec Compare May 19, 2025 09:14
@kou kou added CI: Extra Run extra CI and removed CI: Extra Run extra CI labels May 19, 2025
@kou kou force-pushed the ci-crossbow-cpp-meson branch from db197ec to 2d21358 Compare May 19, 2025 09:16
@kou kou added CI: Extra Run extra CI and removed CI: Extra Run extra CI labels May 19, 2025
@kou

kou commented May 19, 2025

Copy link
Copy Markdown
Member Author

pull_request.types: [labeled] will not work well. We need another approach.

@kou kou force-pushed the ci-crossbow-cpp-meson branch 2 times, most recently from cd9d8d8 to b036227 Compare May 20, 2025 05:58
@kou kou removed the CI: Extra Run extra CI label May 20, 2025
@kou kou force-pushed the ci-crossbow-cpp-meson branch 2 times, most recently from c8212e4 to f55a515 Compare May 20, 2025 06:03
@kou kou added the CI: Extra Run extra CI label May 20, 2025
@kou kou force-pushed the ci-crossbow-cpp-meson branch 2 times, most recently from 24717c1 to c3a2f66 Compare May 20, 2025 06:16
Comment thread .github/workflows/cpp_extra.yml Outdated
@github-actions github-actions Bot added awaiting changes Awaiting changes and removed awaiting committer review Awaiting committer review labels May 20, 2025
@kou kou force-pushed the ci-crossbow-cpp-meson branch from c3a2f66 to 15f4a51 Compare May 22, 2025 03:21
@github-actions github-actions Bot added awaiting change review Awaiting change review and removed awaiting changes Awaiting changes labels May 22, 2025
@kou kou force-pushed the ci-crossbow-cpp-meson branch 2 times, most recently from de096f5 to 5451478 Compare May 22, 2025 05:40
@kou kou force-pushed the ci-crossbow-cpp-meson branch from 70b1a97 to 14664aa Compare May 26, 2025 04:18
@github-actions github-actions Bot added awaiting change review Awaiting change review and removed awaiting changes Awaiting changes labels May 26, 2025
@kou kou force-pushed the ci-crossbow-cpp-meson branch 2 times, most recently from 1a86e48 to 3d0628c Compare May 26, 2025 07:28
@WillAyd

WillAyd commented May 26, 2025

Copy link
Copy Markdown
Contributor

@WillAyd Can we avoid changing the source directory by Meson? Should we use --wrap-mode=nodownload in CI?

Hmm well I think this will prevent the dependencies from being automatically downloaded, but then you will have to have the dependency installed in the CI container some other way, so not sure that is really the fix.

IIUC the error message is coming from tests that belong to bzip2. I think we can avoid running subproject tests if we change:

meson test

to

meson test "arrow*"

@kou

kou commented May 26, 2025

Copy link
Copy Markdown
Member Author

Can we disable bundled bzip2 tests entirely?

In general, we don't want to build tests of bundled dependencies. For example:

@kou

kou commented May 26, 2025

Copy link
Copy Markdown
Member Author

https://github.com/mesonbuild/wrapdb/blob/c068c1051eaf88b7c00618295846ddd94467b272/subprojects/packagefiles/bzip2/meson.build#L27-L106

It seems that we can't disable tests...

@kou kou force-pushed the ci-crossbow-cpp-meson branch from 3d0628c to aa2548d Compare May 26, 2025 21:36
@WillAyd

WillAyd commented May 26, 2025

Copy link
Copy Markdown
Contributor

Yep exactly - I can try to upstream a change to the wrapdb, but in its current state you cannot

@WillAyd

WillAyd commented May 26, 2025

Copy link
Copy Markdown
Contributor

Though interestingly the bzip2 project has its own CMake and Meson configuration files upstream, and neither of them provide an option to disable tests:

https://github.com/libarchive/bzip2/blob/1ea1ac188ad4b9cb662e3f8314673c63df95a589/CMakeOptions.txt

We can probably start by requesting an option in that project directly

@kou

kou commented May 26, 2025

Copy link
Copy Markdown
Member Author

It makes sense. Could you do it?

@WillAyd

WillAyd commented May 27, 2025

Copy link
Copy Markdown
Contributor

Sure - see https://gitlab.com/bzip2/bzip2/-/issues/59

@kou

kou commented May 27, 2025

Copy link
Copy Markdown
Member Author

It seems that we can use meson test --suite arrow to ignore bzip2 tests.

This is ready.

I'll merge this in a few days if nobody objects it.

Comment thread ci/scripts/cpp_build.sh Outdated
Comment on lines 290 to 294

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this still required?

Comment thread ci/scripts/cpp_test.sh Outdated
@github-actions github-actions Bot added awaiting changes Awaiting changes and removed awaiting change review Awaiting change review labels May 27, 2025

@WillAyd WillAyd left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@kou kou force-pushed the ci-crossbow-cpp-meson branch from aa2548d to 0816cd8 Compare May 27, 2025 23:52
@github-actions github-actions Bot added awaiting change review Awaiting change review awaiting changes Awaiting changes and removed awaiting changes Awaiting changes awaiting change review Awaiting change review labels May 27, 2025
@kou

kou commented May 30, 2025

Copy link
Copy Markdown
Member Author

No objection. I'll merge this.

@kou kou merged commit b8c9f76 into apache:main May 30, 2025
34 of 38 checks passed
@kou kou deleted the ci-crossbow-cpp-meson branch May 30, 2025 02:43
@kou kou removed the awaiting changes Awaiting changes label May 30, 2025
@conbench-apache-arrow

Copy link
Copy Markdown

After merging your PR, Conbench analyzed the 4 benchmarking runs that have been run so far on merge-commit b8c9f76.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details. It also includes information about 2 possible false positives for unstable benchmarks that are known to sometimes produce them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CI: Extra Run extra CI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants